Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add code format check as GitHub workflow. #31

Merged
merged 3 commits into from
Feb 2, 2024
Merged

Conversation

karnkaul
Copy link
Member

@karnkaul karnkaul commented Feb 2, 2024

Adds tools/format_code.sh which runs clang-format on all source files in DogTales/.
Note: On Windows this will need to be run in git bash.

Also adds a GitHub workflow - format-check, which runs the script above and fails if it results in modified files.

The first commit demonstrates such a failure, the next commit fixes it.

Once this is merged into main, format-check will be made a required check for PR merges.

Note: click "Show all checks" on a PR to expand the list, if it is collapsed.

Screenshot_20240202_153136

Screenshot_20240202_153207

Screenshot_20240202_153249

@karnkaul karnkaul force-pushed the karnage/format-check branch from d048b2a to 27e2d56 Compare February 2, 2024 03:35
@karnkaul karnkaul marked this pull request as ready for review February 2, 2024 03:37
@karnkaul karnkaul requested review from swagween and A-rms February 2, 2024 03:37
Copy link
Contributor

@swagween swagween left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

code format check approved with one minor question about compact style

@@ -21,8 +21,12 @@ void Player::handle_wall_collision() {
auto const bounce_rect = bave::Rect<>::from_size(m_world_space - m_sprite.get_size(), glm::vec2{0.0f});

// if the sprite's position exceeds the play area, the corresponding velocity component needs to flip.
if (position.x < bounce_rect.top_left().x || position.x > bounce_rect.bottom_right().x) { m_physics.velocity.x *= -0.9f; }
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sometimes I really prefer this compact code style. Not sure if there's a way to allow for it in the formatting check, or perhaps you disagree with using the compact style, in which case, I'm curious to know why!

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah me too, and it's already set to format that way! Unless the line would be too long, which here exceeds the 120 column limit.

@karnkaul karnkaul merged commit a39f6eb into main Feb 2, 2024
3 checks passed
@karnkaul karnkaul deleted the karnage/format-check branch February 2, 2024 14:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants